Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge Cells #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Merge Cells #51

wants to merge 1 commit into from

Conversation

bprasetyo
Copy link

@bprasetyo bprasetyo commented Nov 23, 2020

Add feature merge cells

From this:
unmerge

To this:
merged

@felixbuenemann
Copy link
Owner

@bprasetyo Thanks, can you describe your use case for this feature?

@bprasetyo
Copy link
Author

@felixbuenemann thanks for your reply,
I want to create some report with description at the top of data table, like this:
report

but to keep width of cell for data table, i need to merge column A1 and A2 same with column of data table (A1:C1 and A2:C2) instead of using single cell which make column A4-A16 too wide.

also, like this too:
report2
which merge cell C4:F4

i know gem Axlsx is good for styling an data excel, but in a real case, i have a big data which needed to create to report like this.
so, i want to keep to use this gem because it very fast and low memory usage.

@sandstrom
Copy link
Contributor

I understand if people disagree, but I think the pros of adding this (more capability) should be weighted against added complexity. The primary reason being to keep the complexity down, which makes the project easier to maintain over time.

Maybe it would make sense to add a 'discussions' tab to this repo (github feature) and gauge interested before certain capabilities are added?

@julik
Copy link
Collaborator

julik commented Jan 17, 2025

With merge_cells (the proposed API) I do see a couple of possible issues:

  • You can specify cells which have data. Imagine you merge A1:C1 but previously you have output data into B1. What does Excel do with that, given that one of the cells should end up providing the value for the merged cell?
  • Cell ranges are used - this is OK but a mistake can be made if the syntax is not correct. I would either validate syntax for the merged row addresses or use a Ruby range.

@sandstrom it does seem like a nice addition and I don't see how it causes a lot of cognitive load on the user if you do not have to use it?

@sandstrom
Copy link
Contributor

sandstrom commented Jan 17, 2025

@julik I'm not concerned about cognitive load on the user. I'm thinking about maintenance for this project, and that feature requests with zero upvotes (a single person has suggested it) are often not worth the effort.

The benefit with a discussions tab is that one can measure interest with e.g. upvotes.

Obviously as a project maintainer (I'm not a maintainer here) you can always choose to implement anything. It's just a pattern that some open-source projects take on a scope that's too large, and then eventually becomes impossible to maintain.

https://opensource.guide/best-practices/#learning-to-say-no

@julik
Copy link
Collaborator

julik commented Jan 17, 2025

Thank you for your perspective. I have spoken to Felix and I am co-maintainer on this library as of this year. I agree with you that learning to say "no" to feature proposals / feature requests is an important virtue - and even more so with open-source than with commercial software, as the feature proposal can come in the form of already accomplished changeset - which makes it ever more tempting to just accept.

That said, this feature does seem small and contained enough that it should not bee to much of a burden going forward.

@felixbuenemann
Copy link
Owner

The main goal of this gem ist to dump rows of data to an Excel spreadsheet similar to the csv gem, so I don’t think we need this feature. Also Excel already overflows text into neighboring columns, if they are empty.

@felixbuenemann
Copy link
Owner

@sandstrom I agree with your view on keeping the feature set focused to reduce the maintenance burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants